Skip to content

Conversation

ozersa
Copy link
Contributor

@ozersa ozersa commented Apr 16, 2025

Add MAX32657 NS board support.

This PR will be enabled after: #88651

Blinky Example Output:
image

@ozersa ozersa mentioned this pull request Apr 16, 2025
Copy link

github-actions bot commented Apr 16, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
trusted-firmware-m zephyrproject-rtos/trusted-firmware-m@6473899 zephyrproject-rtos/trusted-firmware-m@c150f48 (main) zephyrproject-rtos/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@@ -2,4 +2,5 @@
# SPDX-License-Identifier: Apache-2.0

config BOARD_MAX32657EVKIT
select SOC_MAX32657 if BOARD_MAX32657EVKIT_MAX32657
select SOC_MAX32657 if BOARD_MAX32657EVKIT_MAX32657 || \
BOARD_MAX32657EVKIT_MAX32657_NS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix alignment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My local tab size was 4, so missed this one, updated it as 8 and fixed. thanks.

BOARD: max32657evkit/max32657/ns
================================

The ``max32657evkit/max32657/ns`` board configuration is used to build the secure firmware
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

board target, no such term as board configuration in zephyr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to classify them to simplify readability. I can update it if require?

  • BOARD: max32657evkit/max32657
  • BOARD: max32657evkit/max32657/ns

@ozersa
Copy link
Contributor Author

ozersa commented May 13, 2025

@tomi-font
Copy link
Contributor

Seems below commit break the TF-M builds for all boards, fyi.

It's happening (only) on your PR because that PR was merged while #89370 is still waiting for @stephanosio's approval. I'd say either wait for it to be merged, or cherry pick that PR into your PR if you want to see CI results, but anyway in the end the other PR will have to go in first.

ozersa added 2 commits May 14, 2025 12:26
Update tf-m node to get MAX32657 support

Signed-off-by: Sadik Ozer <[email protected]>
This commit adds MAX32657 Non-Secure peripheral support

Signed-off-by: Sadik Ozer <[email protected]>
@ozersa ozersa force-pushed the add-MAX32657-NS branch from 33e8d4e to 3a9642c Compare May 14, 2025 09:27
@ozersa
Copy link
Contributor Author

ozersa commented May 14, 2025

Rebased onto main to get #89370

@ozersa ozersa force-pushed the add-MAX32657-NS branch from 3a9642c to db6acb4 Compare May 14, 2025 10:42
Comment on lines 310 to 311
:kconfig:option:`CONFIG_TRUSTED_EXECUTION_SECURE` to ``y`` and
:kconfig:option:`CONFIG_BUILD_WITH_TFM` to ``n``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added.

BOARD: max32657evkit/max32657/ns
================================

The ``max32657evkit/max32657/ns`` target is used to build the secure firmware
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not just board or target. Board target.

Suggested change
The ``max32657evkit/max32657/ns`` target is used to build the secure firmware
The ``max32657evkit/max32657/ns`` board target is used to build the secure firmware

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thanks.

ozersa and others added 8 commits May 14, 2025 14:18
This commit enables max32657 NS board support

To build:
- west build -b max32657evkit/max32657/ns -p

Signed-off-by: Sadik Ozer <[email protected]>
MAX32657 has one UART interface,
It can be used either on TF-M or Zephyr
Enabling debug (TFM_SPM_LOG_LEVEL || TFM_PARTITION_LOG_LEVEL)
will transfer it to the TF-M side
Disabling TF-M debug will transfer it to the Zephyr side.

This commit disable TFM debugs

Signed-off-by: Sadik Ozer <[email protected]>
This commit provides flashing tfm_merged.hex (includes
mcuboot+tfm+ns_image) on default.

Signed-off-by: Mert Ekren <[email protected]>
JWT test fails on max32657evkit/max32657/ns board, due to
it requries hardware TRNG on TF-M side which not activated
yet until it's issue been fixed it is excluded from automated
test to workflow been succeeded

Signed-off-by: Sadik Ozer <[email protected]>
This commit enables secure_partion sample for max32657evkit NS board

Signed-off-by: Sadik Ozer <[email protected]>
This commit enable psa_protected_storage test for max32657evkit/ns board

Signed-off-by: Sadik Ozer <[email protected]>
Update psa_crypto test not work on MAX32657
This commit enable/disable flags for MAX32657 to
make it works for MAX32657.

Signed-off-by: Sadik Ozer <[email protected]>
Set HAL_ADI_PATH to TF-M uses hal_adi that comes with zephyr.

Signed-off-by: Sadik Ozer <[email protected]>
@ozersa ozersa force-pushed the add-MAX32657-NS branch from db6acb4 to f3058cc Compare May 14, 2025 11:21
Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional changes look good. Didn't check the documentation or DT stuff in detail.

@tomi-font tomi-font requested a review from nordicjm May 14, 2025 11:24
Copy link

@ozersa
Copy link
Contributor Author

ozersa commented May 16, 2025

@nordicjm please revisit.

@tomi-font
Copy link
Contributor

FYI I see he's off this week.

@tomi-font
Copy link
Contributor

@d3zd3z please review.

Copy link
Contributor

@valeriosetti valeriosetti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some minor comment. Not mandatory to address them, but it would be nice to do in case you update the PR for some other reason.

config BUILD_WITH_TFM
default y if TRUSTED_EXECUTION_NONSECURE
help
Auto set WITH_TFM for a Non-Secure version of the board,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Auto set WITH_TFM for a Non-Secure version of the board,
Auto set BUILD_WITH_TFM for a Non-Secure version of the board.

Comment on lines +59 to +68
/*
* slot1_partition: partition@f0000 {
* label = "image-1";
* reg = <0xf0000 DT_SIZE_K(0)>;
* };
* slot1_ns_partition: partition@f0000 {
* label = "image-1-nonsecure";
* reg = <0xf0000 DT_SIZE_K(0)>;
* };
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this slot1_ nodes are somehow related to McuBoot update process, but it would be nice to add a short comment which explains why we have some code commented out and when it might be needed to uncomment it. Wdyt?

@@ -8,6 +8,8 @@ common:
- qemu_x86
extra_configs:
- CONFIG_TEST_RANDOM_GENERATOR=y
platform_exclude:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change is OK, but there's a typo in the commit message: Exlucde --> Exclude.

@kartben kartben merged commit 7898a51 into zephyrproject-rtos:main May 21, 2025
30 checks passed
@ozersa ozersa deleted the add-MAX32657-NS branch May 21, 2025 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants